Fix narrowing with final type objects #20743
Conversation
|
|
||
| if isinstance(typ, UnionType): | ||
| type_ranges = [self.get_type_range_of_type(item) for item in typ.items] | ||
| is_upper_bound = any(t.is_upper_bound for t in type_ranges if t is not None) |
There was a problem hiding this comment.
I added this in https://github.com/python/mypy/pull/20675/files#diff-f96a2d6138bc6cdf2a07c4d37f6071cc25c1631afc107e277a28d5b59fc0ef04R7947 but it's not right, this should always be True
See this test case I mention in the PR description: https://github.com/python/mypy/pull/20675/files#diff-e3de7a75a8a107b4f462b164cdf4945d50505c5e9f7092b753c4add0c01530bbR3021-R3037
for more information, see https://pre-commit.ci
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
| t = get_proper_type(t) | ||
| if isinstance(t, UnionType): | ||
| return [b for a in t.items for b in flatten_types_if_tuple(a)] | ||
| return [UnionType.make_union([b for a in t.items for b in flatten_types_if_tuple(a)])] |
There was a problem hiding this comment.
Huh, why the recursive flatten_types_if_tuple here? Maybe I'm misunderstanding, but my impression was that was for the 2nd arg of isinstance -- in which case this fails at runtime: int | (str,).
There was a problem hiding this comment.
why the recursive flatten_types_if_tuple here?
See #20677
There was a problem hiding this comment.
in which case this fails at runtime: int | (str,)
See #20675
There was a problem hiding this comment.
Ah thanks for the reference. However, I don't see why the UnionType.make_union then... e.g., if I have final class A and isinstance w/ (A,) or (B, C), I would assume the type range for A should not have the is_upper_bound?
There was a problem hiding this comment.
I'm not sure I understand... could you spell out the test case?
There was a problem hiding this comment.
Hmm, I'm unable to articulate that into a test case... However I noticed this seems wrong:
from __future__ import annotations
from typing import final
@final
class A: ...
class B: ...
class C: ...
def f1(x: A | C, t: type[A] | type[B]):
if isinstance(x, t):
reveal_type(x) # N: Revealed type is "__main__.A"
else:
reveal_type(x) # Revealed type is "__main__.A | __main__.C"
Specifically, I think the positive branch should be A | <subclass of C and B>, because t could be type[B]...
There was a problem hiding this comment.
Looks like that isn't relevant to this PR though!
2 participants
Follow up feature request from here: #20675 (comment)
Preserves correct behaviour on this test case: https://github.com/python/mypy/pull/20675/files#diff-e3de7a75a8a107b4f462b164cdf4945d50505c5e9f7092b753c4add0c01530bbR3021-R3037